Skip to content

feat(db): support SearchDirection in QueryTransactionsParams#1536

Merged
adecaro merged 2 commits intohyperledger-labs:mainfrom
EvanYan1024:feat/transaction-query-search-direction
Apr 14, 2026
Merged

feat(db): support SearchDirection in QueryTransactionsParams#1536
adecaro merged 2 commits intohyperledger-labs:mainfrom
EvanYan1024:feat/transaction-query-search-direction

Conversation

@EvanYan1024
Copy link
Copy Markdown
Contributor

@EvanYan1024 EvanYan1024 commented Apr 13, 2026 •
edited
Loading

Closes #1539

Summary

QueryTransactions previously hard-coded ORDER BY stored_at ASC, making it
impossible for callers to request newest-first ordering.

This PR adds a *SearchDirection field (pointer) to QueryTransactionsParams:

  • nil (default): ascending order — preserves backward compatibility with all existing callers
  • &FromLast: descending order (newest first)
  • &FromBeginning: ascending order (oldest first, explicit)

Using a pointer avoids the zero-value ambiguity (FromLast = 0), ensuring
existing code that uses QueryTransactionsParams{} continues to work unchanged.

Changes

  • driver/common.go: Add *SearchDirection field to QueryTransactionsParams
  • sql/common/transactions.go: Add transactionOrderBy() helper that defaults to ASC when direction is nil
  • dbtest/transactions.go: Add SearchDirection ASC/DESC test cases using records with distinct timestamps

Test plan

  • go test passes for memory, SQLite, and PostgreSQL backends locally
  • Unit tests verify both ASC and DESC ordering with deterministic timestamps
  • Mock SQL test (TestQueryTransactions) confirms default query uses ORDER BY stored_at ASC
  • All existing tests pass without modification (backward compatible)

@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented Apr 13, 2026

Hi @EvanYan1024 , thanks for submitting this and for the effort. I'll review ASAP.
Please, open a Github Issue. Many thanks 🙏

@EvanYan1024 EvanYan1024 mentioned this pull request Apr 13, 2026
Closed
@adecaro adecaro self-requested a review April 13, 2026 05:36
@adecaro adecaro self-assigned this Apr 13, 2026
@adecaro adecaro added the db label Apr 13, 2026
@adecaro adecaro added this to the Q2/26 milestone Apr 13, 2026
@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented Apr 13, 2026

Hi @EvanYan1024 , thanks for creating the issue.
This is actually very helpful 🙏

Please, have a look at the unit-tests. There must be something there that if fixed might solve also the integration tests.
Thanks.

@EvanYan1024 EvanYan1024 force-pushed the feat/transaction-query-search-direction branch 2 times, most recently from 4888046 to 27f5385 Compare April 13, 2026 06:42
@EvanYan1024
Copy link
Copy Markdown
Contributor Author

EvanYan1024 commented Apr 13, 2026

Hi @EvanYan1024 , thanks for creating the issue. This is actually very helpful 🙏

Please, have a look at the unit-tests. There must be something there that if fixed might solve also the integration tests. Thanks.

@adecaro Hi，I have fixed all CI errors. thanks

@adecaro adecaro force-pushed the feat/transaction-query-search-direction branch from 27f5385 to 6e1d50a Compare April 13, 2026 08:57
@AkramBitar AkramBitar added the enhancement New feature or request label Apr 13, 2026
adecaro
adecaro reviewed Apr 13, 2026
View reviewed changes
Comment thread token/services/storage/db/driver/common.go Outdated
// SearchDirection is the direction of the search.
// If nil, defaults to FromBeginning (ascending by stored_at) for backward compatibility.
// Set explicitly to override: &FromLast for descending, &FromBeginning for ascending.
SearchDirection *SearchDirection
Copy link
Copy Markdown
Contributor

@adecaro adecaro Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need the pointer here because 0 (the default value) represents searching from last. No?

Copy link
Copy Markdown
Contributor Author

@EvanYan1024 EvanYan1024 Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — removed the pointer. Now it uses plain SearchDirection with the zero value (FromLast) defaulting to DESC, consistent with QueryMovementsParams. Updated and force-pushed. Thanks!

@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented Apr 13, 2026

Hi @EvanYan1024 , I think we can remove the pointer and then we are good.
Thanks for the effort 🙏

@EvanYan1024 EvanYan1024 force-pushed the feat/transaction-query-search-direction branch from 6e1d50a to 4ba718b Compare April 13, 2026 15:06
@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented Apr 13, 2026

@EvanYan1024 , please, run make fmt on the PR 🙏

@EvanYan1024 EvanYan1024 force-pushed the feat/transaction-query-search-direction branch from 71f980b to 07590d2 Compare April 13, 2026 15:11
@adecaro adecaro force-pushed the feat/transaction-query-search-direction branch from 07590d2 to 664bc61 Compare April 13, 2026 15:17
adecaro
adecaro approved these changes Apr 13, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@adecaro adecaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks much @EvanYan1024

@EvanYan1024 EvanYan1024 force-pushed the feat/transaction-query-search-direction branch 2 times, most recently from 0246911 to 1e993ba Compare April 14, 2026 01:58
feat(db): support SearchDirection in QueryTransactionsParams
b11c9f4 
QueryTransactions previously hard-coded ORDER BY stored_at ASC,
making it impossible for callers to request newest-first ordering.

Add a SearchDirection field to QueryTransactionsParams, consistent
with the existing field in QueryMovementsParams. The zero value
(FromLast) produces DESC ordering, matching the convention.

Signed-off-by: Evan <evanyan@sign.global>
@EvanYan1024 EvanYan1024 force-pushed the feat/transaction-query-search-direction branch from 1e993ba to b11c9f4 Compare April 14, 2026 02:05
@EvanYan1024
Copy link
Copy Markdown
Contributor Author

EvanYan1024 commented Apr 14, 2026

@EvanYan1024 , please, run make fmt on the PR 🙏

fixed🙏

@adecaro adecaro merged commit 953ffb6 into hyperledger-labs:main Apr 14, 2026
94 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adecaro adecaro adecaro approved these changes

Assignees

@adecaro adecaro

Labels

db enhancement New feature or request

Projects

None yet

Milestone

Q2/26

Development

Successfully merging this pull request may close these issues.

QueryTransactionsParams lacks SearchDirection support (hard-coded ASC ordering)

3 participants

@EvanYan1024 @adecaro @AkramBitar